-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add dismissable Notice to message the rename of Deposits to Payout #9680
Add dismissable Notice to message the rename of Deposits to Payout #9680
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: +6.61 kB (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
…utomattic/woocommerce-payments into add/9673-payouts-rename-notice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change on composer.lock
?
/** | ||
* Internal dependencies | ||
*/ | ||
import { useDeposits } from 'wcpay/data'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and the spotlight UI shows up when I open the deposits list page. I asked if we should also show it on the deposit details page.
…osit details page.
useEffect( () => { | ||
if ( ! isPayoutsRenameNoticeDismissed ) { | ||
setShowTour( true ); | ||
} | ||
}, [ isPayoutsRenameNoticeDismissed ] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this does not have any effect because the value of isPayoutsRenameNoticeDismissed
never changes.
What do you think @Jinksi ?
I will merge this PR because functionality works as expected but we can use a follow up PR to improve here and there, for example #9680 (comment). Also, we might need more unit test for this component. Currently, we test whether the component renders or not but does not test interaction, ie when the close button is clicked etc. |
@shendy-a8c Should this notice display earlier? If the merchant is viewing the deposits list page, they have already found the changed link in sidebar. My understanding of this spotlight notice is to inform the user that payouts has changed, so they can find their way around the new UI (e.g. they might be confused, not seeing Aha, I see @rogermattic from issue:
@shendy-a8c or @jessy-p can you confirm when the notice displays. If we need to follow up, open an issue, or if the PR description is inaccurate, please update (and maybe update the issue too, since its description could be clearer). Thank you! |
Following @rogermattic's suggestion #9673 (comment), I thought it should show on payment overview, deposits list, and deposit details page. But!! Now that I re-read it again, the "dashboard" mentioned in "when the user opens the dashboard in general for the first time" might refer to WooCommerce dashboard or even WordPress dashboard??? I was thinking Payments overview is the "dashboard" referred there, aka WooPayments' dashboard. |
Started a discussion here p1731322836005409-slack-C02BW3Z8SHK. |
Summarising the outcome …
Also it's not practical to add it to other places in admin dashboard, and might feel spammy. |
Fixes #9673
PENDING
get final illustration and copy - Ref [final copy provided]testsChanges proposed in this Pull Request
Adds a one-time dismissible spotlight notice to the Payouts page when user opens either the payments overview page, deposits list page, or deposit details page.
Testing instructions
wcpay_payouts_rename_notice_dismissed
doesn't exist either by runningdelete_option( 'wcpay_payouts_rename_notice_dismissed' )
on WP Console or delete it from yourwp_options
table.npm run changelog
to add a changelog file, choosepatch
to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge